Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dask COUNT queries on windows #2210

Merged
merged 1 commit into from
Feb 20, 2025
Merged

Fix dask COUNT queries on windows #2210

merged 1 commit into from
Feb 20, 2025

Conversation

mivds
Copy link
Contributor

@mivds mivds commented Feb 13, 2025

This commit fixes an issue in the dask cursor that would cause it to return None instead of 0 when performing a COUNT() query on Windows. dask-sql returns an empty data frame when there are no matches. This is normally translated to 0 in the DaskCursor class when using fetchone with a numeric result.

Unfortunately the logic to detect a numeric result did not behave correctly on Windows. The dtype used is int64, which on linux appears to compare equal to int but does not compare equal on Windows.

This commit uses np.issubdtype to have a more robust check for integer types.

This commit fixes an issue in the dask cursor that would cause it to
return `None` instead of `0` when performing a COUNT() query on Windows.
`dask-sql` returns an empty data frame when there are no matches. This
is normally translated to 0 in the DaskCursor class when using fetchone
with a numeric result.

Unfortunately the logic to detect a numeric result did not behave
correctly on Windows. The dtype used is int64, which on linux appears to
compare equal to int but does not compare equal on Windows.

This commit uses `np.issubdtype` to have a more robust check for integer
types.
@CLAassistant
Copy link

CLAassistant commented Feb 13, 2025

CLA assistant check
All committers have signed the CLA.

@m1n0 m1n0 merged commit 08e9ef8 into main Feb 20, 2025
15 checks passed
@m1n0 m1n0 deleted the fix/dask-select-count-windows branch February 20, 2025 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants